refactor(swarm): express dial logic linearly#3253
Conversation
|
With this refactoring, we are not executing certain checks more thoroughly. For example, we make all addresses to be dialed unique and filter out the ones we listen on. Previously, those was only true for the addresses coming from I think the current version is more correct so I pushed a hack that fixes the test. Alternatively, we could also change the test (and perhaps remove the other code that checks for self dials?). |
No longer dialing duplicate addresses would fix #3209. Not doing it everywhere but in |
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
All in all a nice clean-up. Thanks @thomaseizinger. Happy my ugly deeply nested method is gone.
I don't think this patch fixed #3209 because we are doing the deduplication before we ensure the address has a Let's re-open #3209 for now and confirm with a test that it is actually fixed. |
2 participants
Description
Previously, the logic within
Swarm::dialinvolved fairly convolutedmatchexpressions. This patch refactors this function to use new utility functions introduced onDialOptsto handle one concern at a time.This has the advantage that we are covering slightly more cases now. Because we are parsing the
PeerIdonly once at the top, checks like banning will now also act on dials that specify thePeerIdas part of the/p2pprotocol.Notes
I am touching this function as part of #3099 and found it hard to integrate the new callbacks into it.
Links to any relevant issues
Open Questions
Change checklist